-
Notifications
You must be signed in to change notification settings - Fork 28
feat: add method to deallocate reactNativeFactory instance for iOS #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add method to deallocate reactNativeFactory instance for iOS #137
Conversation
okwasniewski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! 🚀
Overall looks good, left couple of questions.
c11f9c0 to
5bef4b4
Compare
|
@okwasniewski, I refactored the factory a bit and added some additional checks due to Cursor Bugbot comments. When it comes to the Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-09-24.at.22.52.53.mp4 |
|
@californiandreamer This looks like a expected behavior if you stop react native, let's ship it. |
ios/ReactNativeBrownfield.swift
Outdated
| return | ||
| } | ||
|
|
||
| guard let factory = reactNativeFactory else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: guard let reactNativeFactory else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would skip for now. The guard let reactNativeFactory else would shadow the property with a let, preventing the later reactNativeFactory = nil reset in this method
5bef4b4 to
aab1339
Compare
|
@okwasniewski I force pushed the updated version of my commits due to the last changes in the rootViewFactory, addressing your comment regarding extra notification dispatch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| * @param launchOptions Launch options, typically passed from AppDelegate. | ||
| */ | ||
| @objc public func startReactNative(onBundleLoaded: (() -> Void)?, launchOptions: [AnyHashable: Any]?) { | ||
| storedLaunchOptions = launchOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Late Option Updates Cause React Native Inconsistency
The storedLaunchOptions assignment happens before the guard check that returns early if reactNativeFactory already exists. This allows launch options to be modified after React Native initialization, potentially causing inconsistent state when view() uses the updated options with an already-initialized factory.
Summary
RCTHost).rootViewFactoryto avoid retaining the old runtime.startReactNative()API remains;view()now self‑heals if RN was previously stopped.Test plan
stopReactNativecalled before RN was initialize)Note
Adds an iOS
stopReactNative()API to tear down the RN runtime, simplifies factory creation/usage, and updates docs and the Swift example.ios/ReactNativeBrownfield.swift):stopReactNative()invalidates the bridge, removes observers, and clears the factory; safe to call multiple times.rootViewFactorywith a lazyfactorycreator;view(...)now resolves viafactory;startReactNative(..., launchOptions:)simplified.stopReactNativereference, description, and examples indocs/SWIFT.mdanddocs/OBJECTIVE_C.md.example/swift/App.swift):ReactNativeBrownfield.shared.stopReactNative(); minor UI tweaks.Written by Cursor Bugbot for commit aab1339. This will update automatically on new commits. Configure here.